Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow the BAM index to be up to 5 seconds older than the BAM before e… #1634

Merged
merged 1 commit into from
Nov 16, 2022
Merged

Allow the BAM index to be up to 5 seconds older than the BAM before e… #1634

merged 1 commit into from
Nov 16, 2022

Conversation

tfenne
Copy link
Member

@tfenne tfenne commented Nov 16, 2022

…mitting the "WARNING: BAM index file ... is older than BAM " message.

Description

HTSJDK itself when writing a BAM and indexing it will frequently leave an mtime on the index that is slightly older than the BAM itself, and this causes a lot of spurious warnings. This is a simple attempt to fix those.

Things to think about before submitting:

  • Make sure your changes compile and new tests pass locally.
  • Add new tests or update existing ones:
    • A bug fix should include a test that previously would have failed and passes now.
    • New features should come with new tests that exercise and validate the new functionality.
  • Extended the README / documentation, if necessary
  • Check your code style.
  • Write a clear commit title and message
    • The commit message should describe what changed and is targeted at htsjdk developers
    • Breaking changes should be mentioned in the commit message.

@tfenne
Allow the BAM index to be up to 5 seconds older than the BAM before e…
9351b1c 
…mitting the "WARNING: BAM index file ... is older than BAM " message.

HTSJDK itself when writing a BAM and indexing it will frequently leave an mtime on the index that is slightly older than the BAM itself, and this causes a lot of spurious warnings.  This is a simple attempt to fix those.
@tfenne tfenne self-assigned this Nov 16, 2022
lbergelson
lbergelson approved these changes Nov 16, 2022
View reviewed changes
Copy link
Member

@lbergelson lbergelson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. This should help some of the cloud instability issues.

@lbergelson lbergelson merged commit 7346f5b into master Nov 16, 2022
@lbergelson lbergelson deleted the tf_soften_index_older_warning branch November 16, 2022 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lbergelson lbergelson lbergelson approved these changes

@nh13 nh13 Awaiting requested review from nh13

Assignees

@tfenne tfenne

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tfenne @lbergelson